cmd/geth: fix importBlock #2244

weiihann · 2024-02-29T07:18:04Z

Description

This PR fixes a bug when importing blocks in an offline manner (i.e. ./bsc import block_data).

Rationale

Expected Behaviour

Command: ./bsc import block_data

...
INFO [02-29|15:00:58.337] Importing blockchain                     file=0_1000000
INFO [02-29|15:00:58.343] Generated state snapshot                 accounts=13 slots=0 storage=843.00B dangling=0 elapsed=9.620ms
INFO [02-29|15:01:06.365] Imported new chain segment               number=805 hash=544976..99b62e miner=0x685B1ded8013785d6623CC18D214320b6Bb64759 blocks=805 txs=25 mgas=4.490 elapsed=8.$
INFO [02-29|15:01:14.371] Imported new chain segment               number=1600 hash=05b2b9..f21f4e miner=0x35E7a025f4da968De7e4D7E4004197917F4070F1 blocks=795 txs=12 mgas=1.240 elapsed=8$
INFO [02-29|15:01:22.376] Imported new chain segment               number=2465 hash=81374e..879eed miner=0x68Bf0B8b6FB4E317a0f9D6F03eAF8CE6675BC60D blocks=865 txs=0  mgas=0.000 elapsed=8$
INFO [02-29|15:01:22.654] Imported new chain segment               number=2500 hash=717e6a..0b30f9 miner=0x2a7cdd959bFe8D9487B2a43B33565295a698F7e2 blocks=35  txs=0  mgas=0.000 elapsed=2$
...

Actual Behaviour

Command: ./bsc import block_data

...
INFO [02-29|14:55:53.094] Importing blockchain                     file=0_1000000
INFO [02-29|14:55:53.098] Generated state snapshot                 accounts=13 slots=0 storage=843.00B dangling=0 elapsed=10.023ms
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x10516ce80]

goroutine 1 [running]:
github.com/ethereum/go-ethereum/internal/ethapi.(*BlockChainAPI).Call(0x0, {0x106ed6c28, 0x14001484c30}, {0x0, 0x140015af188, 0x140016f1c90, 0x0, 0x0, 0x0, 0x0, ...}, ...)
	github.com/ethereum/go-ethereum/internal/ethapi/api.go:1221 +0x60
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).getCurrentValidatorsBeforeLuban(0x1400072f800, {0xd3, 0xe8, 0x6f, 0xee, 0x61, 0xcc, 0x6a, 0x89, 0xaf, ...}, ...)
	github.com/ethereum/go-ethereum/consensus/parlia/lubanFork.go:37 +0x32c
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).getCurrentValidators(0x1400072f800, {0xd3, 0xe8, 0x6f, 0xee, 0x61, 0xcc, 0x6a, 0x89, 0xaf, ...}, ...)
	github.com/ethereum/go-ethereum/consensus/parlia/parlia.go:1527 +0x544
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).verifyValidators(0x1400072f800, 0x14002317680)
	github.com/ethereum/go-ethereum/consensus/parlia/parlia.go:988 +0xdc
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).Finalize(0x1400072f800, {0x106ee1230, 0x14000320c00}, 0x14002317680, 0xcb57d9960655a450?, 0x3af00049?, {0x1c9c380?, 0x140028c6be0?, 0x5f49cd4b?}, {0x140028c6c20?, ...}, ...)
	github.com/ethereum/go-ethereum/consensus/parlia/parlia.go:1114 +0x268
github.com/ethereum/go-ethereum/core.(*StateProcessor).Process(0x14001486b80, 0x1400150df40, 0x14002214780, {{0x0, 0x0}, 0x0, 0x0, 0x0, {0x0, 0x0, ...}})
	github.com/ethereum/go-ethereum/core/state_processor.go:136 +0xdcc
github.com/ethereum/go-ethereum/core.(*BlockChain).insertChain(0x14000320c00, {0x14001f85000?, 0x9c4, 0x9c4}, 0x1)
	github.com/ethereum/go-ethereum/core/blockchain.go:2049 +0x1650
github.com/ethereum/go-ethereum/core.(*BlockChain).InsertChain(0x14000320c00, {0x14001f85000?, 0x9c4, 0x9c4})
	github.com/ethereum/go-ethereum/core/blockchain.go:1827 +0x82c
github.com/ethereum/go-ethereum/cmd/utils.ImportChain(0x0?, {0x16ba23777, 0x9})
	github.com/ethereum/go-ethereum/cmd/utils/cmd.go:218 +0x528
main.importChain(0x140015b7480)
	github.com/ethereum/go-ethereum/cmd/geth/chaincmd.go:493 +0x2bc
github.com/ethereum/go-ethereum/internal/flags.MigrateGlobalFlags.func2.1(0x14000661b68?)
	github.com/ethereum/go-ethereum/internal/flags/helpers.go:91 +0x38
github.com/urfave/cli/v2.(*Command).Run(0x108505cc0, 0x140015b7480, {0x140016c0ce0, 0x2, 0x2})
	github.com/urfave/cli/[email protected]/command.go:274 +0x730
github.com/urfave/cli/v2.(*Command).Run(0x1400056c160, 0x14001502440, {0x1400017c000, 0x9, 0x9})
	github.com/urfave/cli/[email protected]/command.go:267 +0x940
github.com/urfave/cli/v2.(*App).RunContext(0x14001522000, {0x106ed6ad8?, 0x108578740}, {0x1400017c000, 0x9, 0x9})
	github.com/urfave/cli/[email protected]/app.go:332 +0x518
github.com/urfave/cli/v2.(*App).Run(...)
	github.com/urfave/cli/[email protected]/app.go:309
main.main()
	github.com/ethereum/go-ethereum/cmd/geth/main.go:289 +0x4c

The error above is primarily triggered by the following function:

bsc/consensus/parlia/parlia.go

Lines 993 to 998 in bd13416

    
           func (p *Parlia) verifyValidators(header *types.Header) error { 
        
           	if header.Number.Uint64()%p.config.Epoch != 0 { 
        
           		return nil 
        
           	} 
        
           	newValidators, voteAddressMap, err := p.getCurrentValidators(header.ParentHash, new(big.Int).Sub(header.Number, big.NewInt(1)))

Essentially, for every epoch, the Parlia consensus engine would verify the validators set by calling an RPC method. However, when using the ./bsc import block_data tool, the RPC API is not available as it purely does block importing and execution. Hence, this results in the error as shown above.

There are a few solutions to this problem:

Re-engineer the getCurrentValidators method to not use the RPC method. We may risk breaking crucial components with this approach.
Launch the node alongside the RPC API when executing the ./bsc import tool. We only need the RPC API, so other services launched are trivial and add unnecessary processing overhead.
Simply add an option in Parlia to skip verifying validators.

The third option is selected. While it may not be the most elegant, it ensures that this tool is working properly with a minimal change to the codebase, and without breaking things.

Changes

Add a new field noVerifyValidators to Parlia

zzzckck · 2024-03-21T07:57:43Z

There are a few solutions to this problem:
1.Re-engineer the getCurrentValidators method to not use the RPC method. We may risk breaking crucial components with this approach.
2.Launch the node alongside the RPC API when executing the ./bsc import tool. We only need the RPC API, so other services launched are trivial and add unnecessary processing overhead.
3.Simply add an option in Parlia to skip verifying validators.

///===== Actually, I would prefer Solution 1 or Solution 2 if possible.

weiihann · 2024-03-29T08:35:11Z

Went with the second approach. UX is slightly worse because logs related to starting a node will also appear. But the good thing is that code changes are minimal, no need to modify major components and there is not much performance impact.

cmd/geth/chaincmd.go

zzzckck · 2024-04-11T06:15:45Z

We can change the commit message now.

weiihann force-pushed the develop/fix-import-block branch 2 times, most recently from cc6c52e to 4ce7033 Compare March 29, 2024 08:33

weiihann requested review from zzzckck and unclezoro as code owners March 29, 2024 08:33

zzzckck reviewed Apr 3, 2024

View reviewed changes

cmd/geth/chaincmd.go Show resolved Hide resolved

weiihann force-pushed the develop/fix-import-block branch from 4ce7033 to 100f33f Compare April 3, 2024 06:10

zzzckck approved these changes Apr 11, 2024

View reviewed changes

cmd/geth: fix importBlock

6eed590

weiihann force-pushed the develop/fix-import-block branch from 100f33f to 6eed590 Compare April 15, 2024 07:04

weiihann changed the title ~~cmd, parlia: option to disable verify validators~~ cmd/geth: fix importBlock Apr 15, 2024

zlacfzy approved these changes Apr 15, 2024

View reviewed changes

zzzckck merged commit 837de88 into bnb-chain:develop Apr 15, 2024
7 checks passed

This was referenced Apr 26, 2024

release: prepare for release v1.4.6 #2426

Merged

Draft release v1.4.6 #2427

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/geth: fix importBlock #2244

cmd/geth: fix importBlock #2244

weiihann commented Feb 29, 2024

zzzckck commented Mar 21, 2024

weiihann commented Mar 29, 2024

zzzckck commented Apr 11, 2024

	func (p Parlia) verifyValidators(header types.Header) error {
	if header.Number.Uint64()%p.config.Epoch != 0 {
	return nil
	}

	newValidators, voteAddressMap, err := p.getCurrentValidators(header.ParentHash, new(big.Int).Sub(header.Number, big.NewInt(1)))

cmd/geth: fix importBlock #2244

cmd/geth: fix importBlock #2244

Conversation

weiihann commented Feb 29, 2024

Description

Rationale

Expected Behaviour

Actual Behaviour

Changes

zzzckck commented Mar 21, 2024

weiihann commented Mar 29, 2024

zzzckck commented Apr 11, 2024